Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

breaking: remove MediaPlaybackRequiresUserAction and update MediaTypesRequiringUserActionForPlayback to proper variable types #797

Merged
merged 2 commits into from
Feb 27, 2020

Conversation

erisu
Copy link
Member

@erisu erisu commented Feb 23, 2020

Motivation and Context

MediaPlaybackRequiresUserAction was replaced with MediaTypesRequiringUserActionForPlayback in a previous PR. Since the MediaTypesRequiringUserActionForPlayback defaults will always exist in defaults.xml, MediaPlaybackRequiresUserAction will never be read. The old preference logic should actually be removed.

Additionally, the old preference MediaPlaybackRequiresUserAction accepted boolean values but the new MediaTypesRequiringUserActionForPlayback preference does not accept boolean. This needs to be corrected to accept proper value types.

Description

This PR removes all of MediaPlaybackRequiresUserAction logic except for the deprecated warning. The warning is used to provide users a migration step. It will be removed in a later release.

This PR has also updated the native code to pass in the correct variable types for the new
MediaTypesRequiringUserActionForPlayback preference option.

Example valid preference option with proper values are:

<preference name="MediaTypesRequiringUserActionForPlayback" value="all" />
<preference name="MediaTypesRequiringUserActionForPlayback" value="audio" />
<preference name="MediaTypesRequiringUserActionForPlayback" value="video" />
<preference name="MediaTypesRequiringUserActionForPlayback" value="none" />

Testing

  • npm t
  • cordova platform add
  • cordova build ios
  • Run in simulator

Checklist

  • I've run the tests to see all new and existing tests pass
  • I've updated the documentation if necessary

@erisu erisu added this to the 6.0.0 milestone Feb 23, 2020
@erisu erisu force-pushed the breaking/MediaPlaybackRequiresUserAction branch 3 times, most recently from 581a1ef to d322947 Compare February 25, 2020 04:51
@codecov-io
Copy link

codecov-io commented Feb 25, 2020

Codecov Report

Merging #797 into master will decrease coverage by 0.16%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #797      +/-   ##
=========================================
- Coverage   74.36%   74.2%   -0.17%     
=========================================
  Files          13      13              
  Lines        1845    1849       +4     
=========================================
  Hits         1372    1372              
- Misses        473     477       +4
Impacted Files Coverage Δ
bin/templates/scripts/cordova/lib/prepare.js 82.05% <0%> (-0.73%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update df33c47...cb8f965. Read the comment docs.

* As the new preference key always exists in "defaults.xml", the old preference key would never be used.
* Updated deprecation warning logic to include "has been" deprecated since it was completely removed. Most cases the warning would be removed as well but the previous releases never properly warn the user. Warning will remain for next major only as a migration step for users.
@erisu erisu force-pushed the breaking/MediaPlaybackRequiresUserAction branch from d322947 to cb8f965 Compare February 25, 2020 09:06
@erisu erisu merged commit d9a9d01 into apache:master Feb 27, 2020
@erisu erisu deleted the breaking/MediaPlaybackRequiresUserAction branch February 27, 2020 06:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants